Skip to content

feat: box environment #155

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 13 commits into
base: l-monninger/environments-api
Choose a base branch
from
Open

feat: box environment #155

wants to merge 13 commits into from

Conversation

l-monninger
Copy link
Contributor

Summary

Provides initial implementation of box Environment. Additionally, provides the option to snapshot from live movement instance into an isolated instance of movement migrator in a manner which partially addresses #136.

Note

There are some more complicated API features that make this slightly more friendly in real live environments. But, I am likely going to wait until there's more general exposure to the problems those address before trying to add the to the box environment. Likely this will depend a bit upon #148.

@andygolay andygolay mentioned this pull request Jun 13, 2025
Open
2 tasks
andygolay
andygolay requested changes Jun 13, 2025
View reviewed changes
Copy link
Collaborator

@andygolay andygolay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting this permission denied error in the snapshot test:

Error: failed to copy dir for MovementMigrator

Caused by:
    Permission denied (os error 13)
test migrator::movement_migrator::tests::test_snapshot ... FAILED

andygolay
andygolay reviewed Jun 13, 2025
View reviewed changes
@l-monninger
Copy link
Contributor Author

It looks like the failure has to do with the user which CI is using when spawning the containers vs. when running the rest of the program. I wonder if try_home_debug would work around this. Probably not, but could be worth a shot.

@l-monninger l-monninger mentioned this pull request Jun 17, 2025
Open
sebtomba
sebtomba reviewed Jun 17, 2025
View reviewed changes
migration/util/migrator-types/src/migrator/movement_migrator/live.rs
Ok(response) => {
if response.status().is_success() {
ready = true;
continue;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears aggressive. Maybe we could add a delay here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, could be good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sebtomba sebtomba sebtomba left review comments

@andygolay andygolay andygolay requested changes

@0xmovses 0xmovses Awaiting requested review from 0xmovses

@areshand areshand Awaiting requested review from areshand

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@l-monninger @sebtomba @andygolay